feat: database crate#225
Merged
Merged
Conversation
heilhead
force-pushed
the
feat/database-crate-rebased
branch
from
1d2388b to
d0c9c70
Compare
heilhead
requested review from
nopestack and
xDarksome
and removed request for
xDarksome
heilhead
force-pushed
the
feat/database-crate-rebased
branch
from
1c9e1be to
e65aa03
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
Adds a new wcn_db crate to provide a standalone database server binary and migrates existing code to use the new wcn_rocks and wcn_logging crates.
Key changes:
- Swapped out
relay_rocksforwcn_rocksacross node and storage layers, and updated TTL methods fromexp/setexp/hexp/hsetexptoget_exp/set_exp/hget_exp/hset_exp. - Renamed error payload fields from
detailstomessagewithwith_messagebuilder methods. - Introduced convenience impls (
from_bytes/as_bytes,from_unix_timestamp_*,to_unix_timestamp_*) forNamespace,RecordExpiration, andRecordVersion.
Reviewed Changes
Copilot reviewed 43 out of 44 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/rpc/src/quic/client.rs | Bug in named-format usage for format!. |
| crates/storage_api2/src/rpc/mod.rs | Renamed RPC error field details → message. |
| crates/rocks/src/db/types/common/iterators.rs | Renamed has_more → has_next in scan result. |
| crates/wcn/src/commands/node/{stop.rs,start.rs,mod.rs} | Updated logger import to wcn_logging. |
| crates/rocks/src/db/types/{string.rs,map.rs,mod.rs} | Unified Record and MapRecord types and renamed methods. |
Comments suppressed due to low confidence (3)
crates/storage_api2/src/rpc/mod.rs:159
- Changing the serialized field name from
detailstomessageis a breaking change for RPC clients. Consider adding a#[serde(rename = "details")]or#[serde(alias = "details")]onmessageto maintain backward compatibility.
message: Option<String>,
crates/rocks/src/db/types/common/iterators.rs:48
- Renaming the scan result field
has_moretohas_nextchanges the public API. Ensure consumers are updated and consider deprecating the old field or documenting the rename.
pub has_next: bool,
crates/rpc/src/quic/client.rs:198
- Using named braces inside
format!without supplying named arguments will fail to compile. Either switch back to positional formatting ("... {} ... {}", peer_id, actual_peer_id) or use named parameters (format!("... {peer_id} ... {actual_peer_id}", peer_id=peer_id, actual_peer_id=actual_peer_id)).
"Invalid peer ID: expected: {peer_id} actual: {actual_peer_id}"
nopestack
reviewed
Jul 6, 2025
|
|
||
| Ok(Self { | ||
| keypair: raw.keypair, | ||
| db_port: raw.db_port.unwrap_or(3010), |
There was a problem hiding this comment.
consider making these default values constants instead.
Contributor
Author
There was a problem hiding this comment.
That would be inconsistent with other crates. If we decide to change the way we initialize these configs, we should update all of the binary crates that initialize this way.
There was a problem hiding this comment.
it can be done progressively though, minor nit though
Member
There was a problem hiding this comment.
We already discussed that multiple times @nopestack and agreed that if the var is being used in one place only it's fine to inline it
heilhead
force-pushed
the
feat/database-crate-rebased
branch
from
3dd2585 to
fd72ff5
Compare
nopestack
reviewed
Jul 7, 2025
heilhead
force-pushed
the
feat/database-crate-rebased
branch
2 times, most recently
from
6d8d954 to
d255003
Compare
xDarksome
reviewed
Jul 7, 2025
xDarksome
approved these changes
Jul 7, 2025
heilhead
force-pushed
the
feat/database-crate-rebased
branch
from
dd757f7 to
ef371dc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This adds
wcn_dbcrate for a standalone database server binary.How Has This Been Tested?
Existing tests and some new ones.
Due Diligence